-
Notifications
You must be signed in to change notification settings - Fork 346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement thiserror
for libcontainer - Part 1
#1876
Conversation
Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
… etc. Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
Signed-off-by: yihuaf <yihuaf@unkies.org>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1876 +/- ##
==========================================
- Coverage 68.16% 67.70% -0.46%
==========================================
Files 122 124 +2
Lines 14011 14205 +194
==========================================
+ Hits 9550 9618 +68
- Misses 4461 4587 +126 |
@@ -8,3 +8,109 @@ pub mod syscall; | |||
pub mod test; | |||
|
|||
pub use syscall::Syscall; | |||
#[derive(Debug, thiserror::Error)] | |||
pub enum SyscallError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@squili May I ask you to review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! I love how you structured the error ADTs!
Thanks @squili |
This is a first attempt to add
thiserror
tolibcontainer
and all comments are welcome. Note, I decide to break this effort into multiple parts because a single PR would be too large to reasonably review. The aim is to start at the bottom turning the errors intothiserror
while keeping the top levelanyhow::Error
until we finish this effort. By keeping the top level asanyhow::Error
for the moment, we can easily turnthiserror
into ananyhow::Error
, but not vice versa.A few notes. I tried my best to create appropriate error, but they are not perfect in any means. I would like in this PR to focus on the broader effort to make the transition, instead of focusing on the specifics of a single error. We can always make additional PR to fix.
I am not sure if I am using the
libcgroups
error correctly. For now, I just turned them into string messages. We may need to spend more effort since it currently only returns astd::error::Error + 'static
trait, rather than a concrete type where to can expose structurally.